Rfc6724 resolver ordering semantics#778
Conversation
Define INTERLEAVE as no-bias and align tests and debug output.
0802ffe to
3a1bd12
Compare
rschmitt
left a comment
There was a problem hiding this comment.
Main thing here is that I'd like to see a proper implementation of INTERLEAVE, more unit test coverage, and an example program we can use for manual testing.
...client5/src/test/java/org/apache/hc/client5/http/Rfc6724AddressSelectingDnsResolverTest.java
Outdated
Show resolved
Hide resolved
httpclient5/src/main/java/org/apache/hc/client5/http/Rfc6724AddressSelectingDnsResolver.java
Outdated
Show resolved
Hide resolved
httpclient5/src/main/java/org/apache/hc/client5/http/AddressSelectingDnsResolver.java
Show resolved
Hide resolved
httpclient5/src/main/java/org/apache/hc/client5/http/Rfc6724AddressSelectingDnsResolver.java
Outdated
Show resolved
Hide resolved
httpclient5/src/main/java/org/apache/hc/client5/http/AddressSelectingDnsResolver.java
Show resolved
Hide resolved
httpclient5/src/main/java/org/apache/hc/client5/http/AddressSelectingDnsResolver.java
Show resolved
Hide resolved
httpclient5/src/main/java/org/apache/hc/client5/http/config/ProtocolFamilyPreference.java
Show resolved
Hide resolved
| InetAddress src = null; | ||
| try (final DatagramSocket s = new DatagramSocket()) { | ||
| s.connect(dest); // does not send packets; OS picks source addr/if | ||
| src = s.getLocalAddress(); |
There was a problem hiding this comment.
You probably want to inject a thing here that you can mock for unit testing purposes, e.g. a Function<InetSocketAddress, InetAddress> (or an equivalent that throws IOException).
...client5/src/test/java/org/apache/hc/client5/http/Rfc6724AddressSelectingDnsResolverTest.java
Outdated
Show resolved
Hide resolved
httpclient5/src/main/java/org/apache/hc/client5/http/Rfc6724AddressSelectingDnsResolver.java
Outdated
Show resolved
Hide resolved
Add manual gated IT to dump DEFAULT vs INTERLEAVE results. Expand unit coverage for scope mapping and core RFC comparison rules.
|
@rschmitt Please do another pass. I think i solve all your remarks |
httpclient5-testing/src/test/java/org/apache/hc/client5/testing/ManualRfc6724ResolverIT.java
Outdated
Show resolved
Hide resolved
httpclient5/src/main/java/org/apache/hc/client5/http/Rfc6724AddressSelectingDnsResolver.java
Outdated
Show resolved
Hide resolved
httpclient5/src/main/java/org/apache/hc/client5/http/AddressSelectingDnsResolver.java
Show resolved
Hide resolved
httpclient5/src/main/java/org/apache/hc/client5/http/Rfc6724AddressSelectingDnsResolver.java
Outdated
Show resolved
Hide resolved
httpclient5/src/main/java/org/apache/hc/client5/http/AddressSelectingDnsResolver.java
Show resolved
Hide resolved
httpclient5/src/main/java/org/apache/hc/client5/http/Rfc6724AddressSelectingDnsResolver.java
Outdated
Show resolved
Hide resolved
| import org.apache.hc.client5.http.SystemDefaultDnsResolver; | ||
| import org.apache.hc.client5.http.config.ProtocolFamilyPreference; | ||
|
|
||
| public final class Rfc6724ResolverExample { |
There was a problem hiding this comment.
src/hc/client # ./run-example.sh Rfc6724ResolverExample www.amazon.com
Host: www.amazon.com
Preference: DEFAULT
IPv6 2600:1409:9800:1680:0:0:0:3bd4
IPv6 2600:1409:9800:168d:0:0:0:3bd4
IPv4 184.27.192.135
src/hc/client # ./run-example.sh Rfc6724ResolverExample www.amazon.com INTERLEAVE
Host: www.amazon.com
Preference: INTERLEAVE
IPv6 2600:1409:9800:168d:0:0:0:3bd4
IPv4 184.27.192.135
IPv6 2600:1409:9800:1680:0:0:0:3bd4
Neat! I'd be cool if I could see the before-and-after (maybe you could intercept the "before" by wrapping the system default resolver, then passing the wrapper in to the RFC 6724 resolver), but it's not essential.
...client5/src/test/java/org/apache/hc/client5/http/Rfc6724AddressSelectingDnsResolverTest.java
Outdated
Show resolved
Hide resolved
...client5/src/test/java/org/apache/hc/client5/http/Rfc6724AddressSelectingDnsResolverTest.java
Outdated
Show resolved
Hide resolved
httpclient5-testing/src/test/java/org/apache/hc/client5/testing/ManualRfc6724ResolverIT.java
Outdated
Show resolved
Hide resolved
|
There are some things from my first review that still haven't been addressed. I suggest going through the comments one by one and hitting "Resolve conversation" as you address each one, so you can track what is and isn't done. |
…ults. Tighten resolver semantics and filtering for HTTP/TCP connect targets.
Hi @rschmitt I believe I’ve addressed all your remarks in the latest update. |
Introduce Rfc6724AddressSelectingDnsResolver and unit tests.
Define INTERLEAVE as “no bias” (preserve RFC6724 order); keep ONLY/PREFER behavior unchanged.